Repository navigation
/doc/core/: document zend_string APIs - #21642
LamentXU123 wants to merge 1 commit into
Conversation
LamentXU123
left a comment
LamentXU123
left a comment
There was a problem hiding this comment.
Just commenting for easier reviewing
| - - Function/Macro [#persistent]_ | ||
| - Description | ||
|
|
||
| - - ``ZSTR_INIT(s, p)`` |
There was a problem hiding this comment.
I think the original doc is wrong. This should be ZSTR_INIT_LITERAL. Is it?
| - Same as ``zend_string_concat2``, but for three character buffers. | ||
|
|
||
| - - ``ZSTR_EMPTY_ALLOC()`` | ||
| - Gets an immutable, empty string. This does not allocate memory. |
There was a problem hiding this comment.
Return would be better in case of Get in functions
| - Decreases the reference count and frees the string if it goes to 0. | ||
|
|
||
| - - ``zend_string_dup(s, p)`` | ||
| - Creates a true copy of the string in a new allocation, except if the string is interned. |
There was a problem hiding this comment.
How does this function related to reference counting? Shouldn't it be in the "create/allocate" section?
|
|
||
| - - ``ZSTR_HASH`` | ||
| - ``Z_STRHASH[_P]`` | ||
| - Computes the string has if it hasn't already been, and returns it. |
There was a problem hiding this comment.
not sure if this fix is right, but the original one is wrong.
| API | ||
| ***** | ||
|
|
||
| The string API is defined in ``Zend/zend_string.h``. It provides a number of functions for creating |
There was a problem hiding this comment.
It do provide other functions except just for creating new strings?
| hashing, and interning helpers. | ||
|
|
||
| .. list-table:: ``zend_string`` creation | ||
| .. list-table:: Creation and allocation APIs |
There was a problem hiding this comment.
Just to make things more accurate here
iluuu1994
left a comment
iluuu1994
left a comment
There was a problem hiding this comment.
Honestly, I wonder of how much use listing all functions and macros is. I think the docs should mainly focus on explaining concepts and core functions. Once you grasp that, you'll understand all the variations.
If we want to document these functions, maybe we should see if we can document them in-source instead and export the synopses somehow?
Anyway, I don't object but I think the wording could be improved.
Also: Yes, I started reviewing this in April. 😄
|
Thanks for reviewing! I am far from a native English user so yeah some wording might be bad. |
524a1b3 to
a364b50
Compare
Co-authored-by: Ilija Tovilo <1752683+iluuu1994@users.noreply.github.com>
|
Hey @iliaal Can you give a quick review to the doc I wrote in this PR? |
I think these are the main questions to answer here. |
I see that in the opposite way. The reason I am writing these doc is to ideally document all useful functions. There are a lot of helpful sources out side (e.g. thephpinternalsbook) to get people learn what the core concepts and function is, however when implementating stuff, there are very little documentation about how other "variations" that might be useful work. The motivation is when I start to contribute to php core, I start from reading books explaining PHP 8's internal concepts and referring to thephpinternalsbook all the time. However when actually writing PRs, I find it difficult to use the already existed APIs (like zend_string APIs) which is mostly undocumented or ill-documented.
Yes. But that's another topic I guess. I am all for "inlining" the documentation but I think this is more like a future scope and needs more discussion. |
3 participants
This PR added (and also fixed) some internal APIs concerning zend_string, cc @iluuu1994 as we privately talk about the doc work before.